Skip to content

Functional update, host and service probes #5653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Feb 5, 2019
Merged

Functional update, host and service probes #5653

merged 29 commits into from
Feb 5, 2019

Conversation

hreintke
Copy link
Contributor

Initial version for review and feedback
In the LEADNS code I added // Functional at the locations I updated

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hreintke I think the approach is mostly ok, except for my comments. I'll be able to answer with more certainty once you have a more complete implementation.

Just keep pushing changes to this PR, and let me know once you want me to review again.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How change in bin size before this change vs. after?

{
std::map<String,String> tempKV;
for (auto kv = p_pMDNSResponder._answerKeyvalue(p_hServiceQuery, p_u32AnswerIndex);kv != nullptr;kv = kv->m_pNext) {
tempKV.insert(std::pair<String,String>(String(kv->m_pcKey),String(kv->m_pcValue)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can't move to a map of const char *, use of emplace(kv->m_pcKey, kv->m_pcValue) would be better here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, I think the map<String, String> is the only pending.

@devyte devyte added this to the 2.5.0 milestone Feb 4, 2019
@hreintke
Copy link
Contributor Author

hreintke commented Feb 4, 2019

@devyte :
The CI failed due to a socket error within the CI proces.
Is there a way to trigger a new CI build ?

@devyte devyte merged commit 82789d2 into esp8266:master Feb 5, 2019
@devyte devyte self-assigned this Feb 5, 2019
@devyte
Copy link
Collaborator

devyte commented Feb 5, 2019

Excellent work everyone!

@hreintke hreintke deleted the FunctionalMDNS branch May 2, 2019 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants